You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Respect --position when moving a widget to another sidebar - #95
When a widget moves to another sidebar with --position=<n>, it lands at position 1 instead of <n> if <n> is the same as its position in the current sidebar:
Expected archives-1 at position 2, but the list shows:
id position
archives-1 1
search-2 2
(archives-1 was at position 2 in sidebar-1.)
move() uses the current index as the new index when --position is missing, and a move to another sidebar then resets the index to 0 when the new index equals the current index. That check also matches a given --position whose value equals the current position, so the position is ignored.
The "add to the top" rule now checks whether --position was given (empty( $assoc_args['position'] )) instead of comparing indexes. A move to another sidebar without --position still puts the widget at the top, and moves within the same sidebar are unchanged.
Testing
Adds a scenario to features/widget.feature, after "Widget CRUD". It moves archives-1 (position 2 in sidebar-1) to sidebar-2 with --position=2 and checks that wp widget list sidebar-2 shows search-2 at 1 and archives-1 at 2.
Without the fix, the scenario fails because wp widget list sidebar-2 shows archives-1 at position 1 and search-2 at 2 (WordPress 7.1.4, MySQL 8.4).
With the fix, features/widget.feature (8 scenarios) passes on WordPress 7.1.4, MySQL 8.4.
PHPCS passes on src/Widget_Command.php. The linter, Gherkin lint and PHPStan pass.
AI assistance: I used Claude Code to find this and write the change; I reviewed and ran everything myself.
Summary by CodeRabbit
Bug Fixes
Widgets moved to a different sidebar without a specified position now appear at the top of the destination sidebar.
Tests
Added coverage verifying the widget’s position and the success message after moving it.
No actionable comments were generated in the recent review.
Walkthrough
The move command now selects the top position for cross-sidebar moves when --position is not provided. A feature scenario checks the response and widget order after moving a widget to position 2 in another sidebar.
Changes: Widget move position
Layer / File(s)
Summary
Cross-sidebar position selection src/Widget_Command.php, features/widget.feature
The command selects the top position for a cross-sidebar move without an explicit --position. The feature scenario checks the success response and verifies the widget positions in the destination sidebar.
The change addresses explicit destination positions, with a scenario checking the resulting order. No merge-blocking risk is identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main behavior change: when moving a widget to another sidebar, the --position value is now respected instead of always placing the widget at the top. The Widget_Command::move() cha…
Linked Issues check
Objective [#94]: when a widget moves to another sidebar with --position=, the widget must land at position , even when equals its current position. The head diff of src/Widget_Command.php ch…
Out of Scope Changes check
The PR changes two files. src/Widget_Command.php changes one condition in move(). It only affects moves between sidebars, which is the behavior in [#94]. features/widget.feature adds one scenario that…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title names the main behavior change: when moving a widget to another sidebar, the --position value is now respected instead of always placing the widget at the top. The Widget_Command::move() change in src/Widget_Command.php makes the top-placing rule depend on whether --position was supplied, and the added features/widget.feature scenario checks the explicit-position move. The title is specific and matches the linked issue.
Full details: Linked Issues check
Explanation
Objective [#94]: when a widget moves to another sidebar with --position=<n>, the widget must land at position <n>, even when <n> equals its current position. The head diff of src/Widget_Command.php changes the reset condition in move() from $new_sidebar_index === $current_sidebar_index to empty( $assoc_args['position'] ). The reset to index 0 now runs only when --position was not supplied. A supplied --position keeps its computed index, so archives-1 (position 2 in sidebar-1) moved with --position=2 keeps index 1 and lands at position 2 in sidebar-2. Objective [#94]: a move to another sidebar without --position still goes to the top. The new condition sets index 0 in that case, which keeps the existing behavior. Moves within the same sidebar are unchanged, because the reset condition requires a different sidebar ID. Automated test: features/widget.feature at head contains the scenario 'Move a widget to a position in another sidebar'. It runs wp widget move archives-1 --sidebar-id=sidebar-2 --position=2, expects 'Success: Widget moved.', and expects wp widget list sidebar-2 to show search-2 at 1 and archives-1 at 2. This matches the issue's expected outcome. Test results reported in the PR description were not re-run in this review. Edge case, not a requirement of #94: --position=0 is treated as not supplied by the existing empty() checks, so it moves the widget to the top. This behavior predates the PR.
Full details: Out of Scope Changes check
Explanation
The PR changes two files. src/Widget_Command.php changes one condition in move(). It only affects moves between sidebars, which is the behavior in [#94]. features/widget.feature adds one scenario that tests the fixed behavior, with a position check for sidebar-2 that matches the issue example. The scenario is a supporting test and does not add unrelated behavior. No other changed code was found in the reviewed diff. The raw summary lists no changes to public declarations. The diff inspection of features/widget.feature was unavailable, so the scenario text was read from the file at head instead. That read showed the scenario as described.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 9da69956-1f2f-4d50-b0a5-f2fe88588b1b
Commits
Reviewing files that changed from the base of the PR and between 6a905f1 and a67f289.
Files selected for processing: 2
features/widget.feature
src/Widget_Command.php
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Thanks for opening this pull request! Please check out our contributing guidelines. We appreciate you taking the initiative to contribute to this project.
Contributing isn't limited to just code. We encourage you to contribute in the way that best fits your abilities, by writing tutorials, giving a demo at your local meetup, helping other users with their support questions, or revising our documentation.
Here are some useful Composer commands to get you started:
To run a single Behat test, you can use the following command:
# Run all tests in a single file
composer behat features/some-feature.feature
# Run only a specific scenario (where 123 is the line number of the "Scenario:" title)
composer behat features/some-feature.feature:123
You can find a list of all available Behat steps in our handbook.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
bug
command:widget
Fixes #94
When a widget moves to another sidebar with
--position=<n>, it lands at position 1 instead of<n>if<n>is the same as its position in the current sidebar:Expected
archives-1at position 2, but the list shows:(
archives-1was at position 2 insidebar-1.)move()uses the current index as the new index when--positionis missing, and a move to another sidebar then resets the index to 0 when the new index equals the current index. That check also matches a given--positionwhose value equals the current position, so the position is ignored.The "add to the top" rule now checks whether
--positionwas given (empty( $assoc_args['position'] )) instead of comparing indexes. A move to another sidebar without--positionstill puts the widget at the top, and moves within the same sidebar are unchanged.Testing
Adds a scenario to
features/widget.feature, after "Widget CRUD". It movesarchives-1(position 2 insidebar-1) tosidebar-2with--position=2and checks thatwp widget list sidebar-2showssearch-2at 1 andarchives-1at 2.wp widget list sidebar-2showsarchives-1at position 1 andsearch-2at 2 (WordPress 7.1.4, MySQL 8.4).features/widget.feature(8 scenarios) passes on WordPress 7.1.4, MySQL 8.4.src/Widget_Command.php. The linter, Gherkin lint and PHPStan pass.AI assistance: I used Claude Code to find this and write the change; I reviewed and ran everything myself.
Summary by CodeRabbit